-
Notifications
You must be signed in to change notification settings - Fork 3.5k
[EH] Remove 'terminating' from exception message #17058
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
emscripten-core#17003 added "terminating" to exception messages, but some people might want to use the message in non-terminating cases. This changes `emscripten_format_exception` to `__get_exception_message`, and separately adds `__get_terminating_exception_message`. Adds two underscores not to clash with the global namespace. `__get_exception_message` prints messages like ``` uncaught exception of type char uncaught exception of type std::runtime_error: abc uncaught exception of type myexception: My exception happened ``` `__get_terminating_exception_message` prints messages like ``` terminating with uncaught exception of type char terminating with uncaught exception of type std::runtime_error: abc terminating with uncaught exception of type myexception: My exception happened ```
|
cc @hoodmane |
|
I am also not sure that the adjective "uncaught" should be in there. We caught it from JavaScript instead of inside of wasm, but depending on your perspective the error may or may not be "uncaught". |
| // the exception, if necessary. By incrementing and decrementing the refcount | ||
| // we trigger the free'ing of the exception if its refcount was zero. | ||
| ___cxa_increment_exception_refcount(p); | ||
| console.log(Module["formatException"](p).replace(/0x[0-9a-f]*/, "xxx")); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@hoodmane By the way I have a question. Is there a reason we can't call this just like
console.log(formatException(p));?
The same for Module["_throw_exc"] in line 1648.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure, try it and see?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From outside the module you would need Module["formatException"].. but for JS that is inside the module (e.g. EM_ASM and EM_JS code) then you can just do formatException.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. Changed them to direct calls.
I think that makes sense. Removed it. |
| return result; | ||
| } | ||
|
|
||
| char* __get_exception_terminate_message(void *thrown_object) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess we don't have any current caller of this yet, but we will do soon (perhaps in debug builds, or some opt-in configuration)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes I'd like to use it later, and this PR doesn't have a direct test for this, but I think holding this small piece off is probably not worth it..
#17003 added "terminating" to exception messages, but some people might
want to use the message in non-terminating cases. This changes
emscripten_format_exceptionto__get_exception_message, andseparately adds
__get_terminating_exception_message. Adds twounderscores not to clash with the global namespace.
__get_exception_messageprints messages like__get_terminating_exception_messageprints messages like